Skip to content

fix(#191): remove stale REPO_DIR from harness configs#192

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/191-remove-harness-repo-dir
Jul 22, 2026
Merged

fix(#191): remove stale REPO_DIR from harness configs#192
rh-hemartin merged 1 commit into
mainfrom
agent/191-remove-harness-repo-dir

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Remove REPO_DIR from forge.github.runner_env in harness/code.yaml and from forge.github.env.runner in harness/fix.yaml. Since fullsend#2774 changed sandbox output extraction to a temp directory, this value pointed to a stale location. The runner (run.go) already injects the correct REPO_DIR via last-value-wins, making the harness entries redundant and confusing.

Post-scripts (post-code.sh, post-fix.sh) are unaffected: they default REPO_DIR with ${REPO_DIR:-repo} and receive the correct value from the runner injection.


Closes #191

Post-script verification

  • Branch is not main/master (agent/191-remove-harness-repo-dir)
  • Secret scan passed (gitleaks — ab29a63c40f2e235c8b1bdd8152d6083a0dddcc3..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Remove REPO_DIR from forge.github.runner_env in harness/code.yaml
and from forge.github.env.runner in harness/fix.yaml. Since
fullsend#2774 changed sandbox output extraction to a temp
directory, this value pointed to a stale location. The runner
(run.go) already injects the correct REPO_DIR via last-value-wins,
making the harness entries redundant and confusing.

Post-scripts (post-code.sh, post-fix.sh) are unaffected: they
default REPO_DIR with ${REPO_DIR:-repo} and receive the correct
value from the runner injection.

Closes #191
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 16, 2026 07:50

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocked until fullsend-ai/fullsend#2774 gets merged.

@rh-hemartin rh-hemartin self-assigned this Jul 20, 2026
@rh-hemartin

Copy link
Copy Markdown
Member

/fs-review

@rh-hemartin
rh-hemartin dismissed their stale review July 20, 2026 11:34

Dep PR merged.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:35 AM UTC · Completed 11:43 AM UTC
Commit: 7d0bfa9 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] harness/code.yaml, harness/fix.yaml — Both changed files are under the harness/ protected path. The PR is properly authorized by issue Remove REPO_DIR from harness RunnerEnv #191 and the rationale is well-explained, but human approval is always required for protected-path changes regardless of context.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 20, 2026
@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 22, 2026
Merged via the queue into main with commit 591bb1f Jul 22, 2026
21 checks passed
@rh-hemartin
rh-hemartin deleted the agent/191-remove-harness-repo-dir branch July 22, 2026 06:34
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:37 AM UTC · Completed 6:45 AM UTC
Commit: 781499d · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #192 — Remove stale REPO_DIR from harness configs

Verdict: Clean workflow with one missed triage step.

This was a well-executed end-to-end workflow with zero rework. The triage agent produced an accurate summary with correct root cause, file identification, and proposed test. The code agent got it right on the first try — a precise 2-line deletion across harness/code.yaml and harness/fix.yaml. The review agent correctly identified the protected-path constraint and applied the requires-manual-review label. The human approved and merged without requesting any changes.

Timeline

  1. Jul 16 07:36 — Human (rh-hemartin) filed issue #191 referencing fullsend-ai/fullsend#2774 as the upstream change that made REPO_DIR stale.
  2. Jul 16 07:37–07:41 — Triage agent ran (run 29480531709). Correctly identified files, root cause, and recommended fix. However, it described #2774 as a completed change without verifying its merge status.
  3. Jul 16 07:41–07:50 — Code agent ran (run 29480763727). Created PR #192 with the correct fix.
  4. Jul 16 09:09 — Human dismissed initial review: "Blocked until fullsend#2774 gets merged."
  5. Jul 20 11:33 — Human manually triggered /fs-review after the dependency landed.
  6. Jul 20 11:35–11:43 — Review agent ran (run 29738998022) with 3 sub-agents (correctness, style-conventions, intent-coherence). Found one medium-severity protected-path finding.
  7. Jul 22 06:32 — Human approved. PR merged at 06:34.

What went well

  • Triage quality: Accurate root cause, correct file identification, actionable recommended fix with proposed test script.
  • Code quality: First-try success. Zero rework. Clean commit message following Conventional Commits.
  • Review quality: Correctly gated on protected-path constraint. Applied the right label.
  • Human-agent collaboration: Smooth handoffs throughout.

Existing issue evidence

  • #301 (lightweight review for trivial changes): This PR is evidence for Review agent should use a lightweight path when it classifies a change as trivial/mechanical #301 — the review agent ran 3 sub-agents over ~8 minutes for a 2-line deletion, finding only a procedural protected-path gate. A lightweight path would have saved tokens here.
  • #150 (suppress low-value compliance findings on trivial config-only PRs): The protected-path finding is technically correct but low-value for a well-authorized 2-line config removal by a repo member.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove REPO_DIR from harness RunnerEnv

1 participant